-
-
Notifications
You must be signed in to change notification settings - Fork 629
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow validators to transform values #2786
base: dev
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this doesn't require many changes to existing validators because they already return the value.
It sounds a bit wrong to use a validator to modify the value, since it is not their intended use. That's not a strong argument, they could be called post_processors. It just makes me wonder if some day we'll realize that we took a shortcut that seemed fine but doesn't actually work for all cases.
Pydantic does it. But in a more complete way ("before", "after", "plain", "wrap"). The implementation in this PR is what Pydantic would call "after". What if tomorrow we want to implement "before", to modify the data before deserialization?
Are we trying to implement pre/post - load/dump but at field level rather than schema level? Should we do that instead?
fwiw sqlalchemy also allow validators to change the value: https://docs.sqlalchemy.org/en/20/orm/mapped_attributes.html#simple-validators
i agree it isn't immediately apparent given the name "validators", but perhaps that's preferable than to adding more API surface? i'm not sure yet.
that's an interesting idea. something like class ArtistSchema(Schema):
name = fields.Str(post_load=(strip_whitespace, uppercase))
email = fields.Str(post_load=(strip_whitespace, uppercase))
# and/or
class ArtistSchema(Schema):
name = fields.Str()
email = fields.Str()
@field_post_load("name", "email")
def strip_whitespace(self, field_name: str, value: str) -> str:
return value.strip() i like this idea, but will have to give it some thought |
i suppose one downside of that would be that |
otoh, it's nice that field_pre/post_load/dump could be added as a non-breaking feature |
Possibly. But the call is not exactly the same since |
Still need to update the docs, but this proves the concept mentioned in #1391 (comment) , where we require that validators return a validated and possibly modified value, allowing users to apply a pipeline of transformations to any field